-
Notifications
You must be signed in to change notification settings - Fork 1
Add Nix dev shell and CI job #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-Authored-By: stringintech <[email protected]>
starius
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🏆
I tested nix develop and make build-kernel NUM_JOBS=1 and make test within it. Works great!
Nix code now looks very compact and readable! Thank you for improving it!
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.23.3' | ||
| go-version: '1.23' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO in this file we need a concrete version for Go (as it was before the commit "Drop patch version from go version")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why you prefer we do not drop the patch version here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it reproducible. 1.23.3 is the exact version of software, 1.23 is a version of language. That is why in go.mod (where we specify the language version) it makes sense to put 1.23 and in CI config (which specifies what exactly to run) - 1.23.3 (or better 1.23.12 which the the latest among 1.23.*).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Thanks. I will apply the patch version 1.23.12 specified here then (compatible with the lock file revision).
| run: nix develop --command make test | ||
|
|
||
| - name: Run linter | ||
| run: nix develop --command make lint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing end-of-line
| - name: Run tests | ||
| run: make test | ||
|
|
||
| nix: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| module github.com/stringintech/go-bitcoinkernel | ||
|
|
||
| go 1.23.3 | ||
| go 1.23 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.